Fix LaunchConfig.grid unit conversion when cluster is set by Copilot · Pull Request #868 · NVIDIA/cuda-python

Copilot · 2025-08-20T16:15:28Z

The LaunchConfig.grid parameter was incorrectly interpreted as blocks instead of clusters when .cluster is specified, causing a mismatch with the expected C++ behavior.

Problem

When using thread block clusters, the grid parameter should represent the number of clusters, not blocks. However, the original implementation treated it as blocks, leading to incorrect launch configurations.

Example from thread_block_cluster.py:

config = LaunchConfig(grid=4, cluster=2, block=32)
# Should mean: 4 clusters × 2 blocks/cluster = 8 total blocks
# But was producing: grid dim: (4, 1, 1) ❌

This doesn't match the C++ equivalent:

cudax::make_hierarchy(cudax::grid_dims(4), cudax::cluster_dims(2), cudax::block_dims(32));
// Where grid_dims(4) means 4 clusters, resulting in 8 total blocks

Solution

Modified _to_native_launch_config() to perform unit conversion when cluster is set, while preserving the original grid values in the LaunchConfig dataclass:

# In _to_native_launch_config()
if config.cluster:
    grid_blocks = (
        config.grid[0] * config.cluster[0],
        config.grid[1] * config.cluster[1],
        config.grid[2] * config.cluster[2]
    )
    drv_cfg.gridDimX, drv_cfg.gridDimY, drv_cfg.gridDimZ = grid_blocks

This approach maintains the dataclass contract - users can still access their original grid values transparently, but the conversion happens when interfacing with the CUDA driver.

Results

LaunchConfig object (preserved):

config = LaunchConfig(grid=4, cluster=2, block=32)
config.grid    # Returns (4, 1, 1) - original user input

Native CUDA configuration (converted):

grid dim: (8, 1, 1)      # Correct: 4 clusters × 2 blocks = 8 blocks
cluster dim: (2, 1, 1)
block dim: (32, 1, 1)

The fix handles all dimensionalities (1D, 2D, 3D) and preserves existing functionality for non-cluster configurations. The enhanced example now demonstrates and validates the correct behavior using pinned memory to verify the grid conversion works as expected.

Fixes #867.

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot · 2025-08-20T16:15:30Z

@leofang 👋 This repository doesn't have Copilot instructions. With Copilot instructions, I can understand the repository better, work faster and produce higher quality PRs.

I can generate a .github/copilot-instructions.md file for you automatically. Click here to open a pre-filled issue and assign it to me. I'll write the instructions, and then tag you for review.

copy-pr-bot · 2025-08-20T16:15:32Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

Co-authored-by: leofang <5534781+leofang@users.noreply.github.com>

leofang

Please add a note to LaunchConfig's docstring, explaining the expectation that the units for the grid / cluster / block are the next object in the hierarchy (for grid it's cluster if present, otherwise block / block / thread)
Please add a release note entry to docs/source/release/0.X.Y.rst, and note that this is a breaking change. Make sure to follow the style we have (by checking how we wrote previous release notes).

Co-authored-by: leofang <5534781+leofang@users.noreply.github.com>

Copilot · 2025-08-20T17:02:08Z

Please add a note to LaunchConfig's docstring, explaining the expectation that the units for the grid / cluster / block are the next object in the hierarchy (for grid it's cluster if present, othe...

Added note to LaunchConfig docstring explaining grid units (clusters when cluster is set, blocks otherwise) and created release note entry in 0.3.3-notes.rst documenting this as a breaking change. Changes committed in b9ca1bc.

leofang · 2025-08-20T19:57:33Z

Please rename this file to literally "0.X.Y-notes.rst", because we don't know yet what's the next release version.

Renamed the file to 0.X.Y-notes.rst in commit 681540b.

leofang · 2025-08-20T20:04:53Z

@copilot please update the PR based on the new review comments

…lify tests Co-authored-by: leofang <5534781+leofang@users.noreply.github.com>

Copilot · 2025-08-20T20:10:01Z

@copilot please update the PR based on the new review comments

Updated the PR based on all review comments in commit 681540b.

leofang

@copilot PTAL

Co-authored-by: leofang <5534781+leofang@users.noreply.github.com>

Copilot · 2025-08-20T20:34:11Z

@copilot PTAL

Updated the PR based on the review feedback. The grid conversion has been moved from LaunchConfig.__post_init__ to _to_native_launch_config() to preserve the dataclass contract while still performing the necessary unit conversion when interfacing with CUDA drivers. Commit: 1c05792

leofang · 2025-08-20T23:34:22Z

/ok to test 50866b8

leofang

@copilot could you check the pre-commit failures? Empty lines should not contain white spaces.

Co-authored-by: leofang <5534781+leofang@users.noreply.github.com>

Copilot · 2025-08-21T00:19:16Z

@copilot could you check the pre-commit failures? Empty lines should not contain white spaces.

Fixed the pre-commit whitespace issues by removing trailing spaces from empty lines in the CUDA C++ code. Commit: 114f0f9

leofang

@copilot

Co-authored-by: leofang <5534781+leofang@users.noreply.github.com>

leofang · 2025-08-21T00:34:35Z

/ok to test 035c91b

leofang · 2025-08-21T00:55:33Z

@pciolkosz could you also take a look? (Can't add you as a reviewer for some reason.)

pciolkosz · 2025-08-21T01:11:46Z

Looks good 👍

github-actions · 2025-08-21T02:04:48Z

Doc Preview CI
Preview removed because the pull request was closed or merged.

Initial plan

6941bcb

Copilot AI assigned Copilot and leofang Aug 20, 2025

github-project-automation Bot added this to CCCL Aug 20, 2025

github-project-automation Bot moved this to Todo in CCCL Aug 20, 2025

Fix LaunchConfig.grid unit conversion when cluster is set

445b3e9

Co-authored-by: leofang <5534781+leofang@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] LaunchConfig.grid is in the wrong unit if .cluster is set~~ Fix LaunchConfig.grid unit conversion when cluster is set Aug 20, 2025

Copilot AI requested a review from leofang August 20, 2025 16:27

leofang requested changes Aug 20, 2025

View reviewed changes

Comment thread cuda_core/tests/test_launcher.py Outdated

Comment thread cuda_core/verify_cluster_fix.py Outdated

github-project-automation Bot moved this from Todo to In Progress in CCCL Aug 20, 2025

Address PR feedback: add docstring, tests, and release notes

b9ca1bc

Co-authored-by: leofang <5534781+leofang@users.noreply.github.com>

Copilot AI requested a review from leofang August 20, 2025 17:03

leofang requested changes Aug 20, 2025

View reviewed changes

Address PR review feedback: fix docstring, rename release notes, simp…

681540b

…lify tests Co-authored-by: leofang <5534781+leofang@users.noreply.github.com>

Copilot AI requested a review from leofang August 20, 2025 20:10

leofang requested changes Aug 20, 2025

View reviewed changes

Comment thread cuda_core/cuda/core/experimental/_launch_config.py Outdated

Move grid conversion from LaunchConfig to native config conversion

1c05792

Co-authored-by: leofang <5534781+leofang@users.noreply.github.com>

leofang added the breaking Breaking changes are introduced label Aug 20, 2025

[pre-commit.ci] auto code formatting

50866b8

This comment has been minimized.

Sign in to view

leofang requested changes Aug 21, 2025

View reviewed changes

Fix whitespace issues: remove trailing spaces from empty lines

114f0f9

Co-authored-by: leofang <5534781+leofang@users.noreply.github.com>

Copilot AI requested a review from leofang August 21, 2025 00:19

leofang requested changes Aug 21, 2025

View reviewed changes

Comment thread cuda_core/docs/source/release/0.X.Y-notes.rst

Add 0.X.Y-notes.rst reference to release.rst for doc rendering

035c91b

Co-authored-by: leofang <5534781+leofang@users.noreply.github.com>

Copilot AI requested a review from leofang August 21, 2025 00:31

leofang requested review from kkraus14 and oleksandr-pavlyk August 21, 2025 00:53

leofang marked this pull request as ready for review August 21, 2025 00:53

pciolkosz approved these changes Aug 21, 2025

View reviewed changes

kkraus14 approved these changes Aug 21, 2025

View reviewed changes

leofang approved these changes Aug 21, 2025

View reviewed changes

github-project-automation Bot moved this from In Progress to In Review in CCCL Aug 21, 2025

leofang merged commit bb1fe80 into main Aug 21, 2025
48 checks passed

leofang deleted the copilot/fix-867 branch August 21, 2025 01:49

github-project-automation Bot moved this from In Review to Done in CCCL Aug 21, 2025

leofang mentioned this pull request Oct 22, 2025

Add a NumPy version check to examples/thread_block_cluster.py #1170

Closed

Uh oh!

Conversation

Copilot AI commented Aug 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Results

Uh oh!

Copilot AI commented Aug 20, 2025

Uh oh!

copy-pr-bot Bot commented Aug 20, 2025

Uh oh!

leofang left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented Aug 20, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

leofang Aug 20, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 20, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

leofang commented Aug 20, 2025

Uh oh!

Copilot AI commented Aug 20, 2025

Uh oh!

leofang left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI commented Aug 20, 2025

Uh oh!

leofang commented Aug 20, 2025

Uh oh!

This comment has been minimized.

leofang left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Aug 21, 2025

Uh oh!

leofang left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

leofang commented Aug 21, 2025

Uh oh!

leofang commented Aug 21, 2025

Uh oh!

pciolkosz commented Aug 21, 2025

Uh oh!

Uh oh!

github-actions Bot commented Aug 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Copilot AI commented Aug 20, 2025 •

edited

Loading